Fix aux-factory lazy bounds.#10
Fix aux-factory lazy bounds.#10corinnebosley merged 2 commits intocorinnebosley:lazy_bounds_testfrom
Conversation
| # extra dimension to make the shape compatible, so | ||
| # we just add an extra 1. | ||
| shape.append(1) | ||
| nd_values = np.array(nd_values) |
There was a problem hiding this comment.
@corinnebosley Maybe this isn't a safe thing to do ... dunno. Most likely, not.
There was a problem hiding this comment.
I'm still not sure about this. It makes me very nervous just whipping out a safety check (wow, how Sheldon is that??).
Although since you have checked through the code path to make sure that there is no way for a non-array (i.e. a value) to appear here, I am a little happier about it.
My trigger finger is itchy, but I also have ants in my pants about this a bit. Not sure what to do...
There was a problem hiding this comment.
We can use test coverage to convince ourselves that his is a sensible change ... Ba-zinga!
There was a problem hiding this comment.
@corinnebosley I'm convinced that at this point, nd_values will always be an array (dask or numpy), as returned by _nd_points or _nd_bounds, and we've now added tests to implicitly check that, so I believe we're safe making this change (which is one of the sources of lazy data being realised)
bjlittle
force-pushed
the
dask-aux-factory
branch
from
f2cddc4 to
c18d796
Compare
| class Test__nd_bounds(tests.IrisTest): | ||
| def test_numpy_scalar_coord__zero_ndim(self): | ||
| bounds = np.arange(2) | ||
| points = np.array(0.5) |
There was a problem hiding this comment.
@bjlittle Would you mind swapping the order of points and bounds on the above two lines please? Nothing functional, just an OCD thing.
| def test_numpy_scalar_coord(self): | ||
| bounds = np.arange(2).reshape(1, 2) | ||
| coord = iris.coords.AuxCoord(np.arange(1), bounds=bounds) | ||
| points = np.array(0.5) |
There was a problem hiding this comment.
@bjlittle Same here. Just feels wrong defining bounds before points.
bjlittle
force-pushed
the
dask-aux-factory
branch
from
c18d796 to
07ee9a0
Compare
2 participants
@corinnebosley I think this is fix to maintain lazy bounds for the derived coordinate ... we need to think about testing this, but we/I/you can do that in a separate PR 😄